Bad code has lots of unique characteristics. In this article, we’ll look at each one and what they are. We look at more general code smells.
Don’t Be Arbitrary
We should structure our code to follow clean code conventions. The code that doesn’t belong in the place shouldn’t be there.
If a team has a convention for the codebase, then everyone should stick to it. Otherwise, it gets messy fast.
Encapsulate Boundary Conditions
Boundary conditions should be put in a function or variable for easy access and understanding.
For example, we should set arr.length — 1 to a variable as follows if we have wanted it to be the end index variable of the loop:
const arr = [1, 2, 3];
const lastIndexOfArray = arr.length - 1;
for (let i = 0; i <= lastIndexOfArray; i++) {
console.log(arr[i]);
}
We can see that once we assigned to arr.length — 1 to the constant lastIndexOfArray, then we know that it is the last index of the array.
We no longer have to think about what it means. From the constant name, we know what it means.
Similarly, we should do it for other boundary cases, so we don’t have to guess why we have +1 or -1 in different places.
Function Should Descend Only One Level of Abstraction
This means that functions should only do one thing. If we require it to do another thing at a different level of abstraction, then we should write a new function and call that.
Since functions should be small and should only do one thing, they shouldn’t be touching different things at different levels.
Keep Configurable Data at High Levels
Keeping configuration data at a high level keep them within our sight. Since we’re using them in many places, it makes sense for them to be at a high level.
If we put them at a high level, then they’re also easy to change.
Avoid Transitive Navigation
We should avoid code structure where we have A referencing B and B referencing C.
It’s harder on our brains to understand this kind of navigation. Also, it exposes us to more code that is coupled together.
We shouldn’t expose code that we don’t want to expose.
For example, something like:
foo.getBar().getBaz().doSomething();
The code above is bad because we have to get the Bar instance with getBar, and then with the Bar instance, we have to get the Baz instance with getBaz. Then we finally call doSomething on the Baz instance.
That’s fragile code, because if anything in the chain breaks, then the whole thing breaks.
Any of them changing will become an issue. We should take out this transitive navigation code by changing the implementation so that we don’t have this kind of structure.
Choose Descriptive Names
Naming things descriptively is important. Names tell us a lot about what a piece of code is doing. Therefore, we should choose names that tells what it’s storing or doing.
Names like x and y aren’t good names for variables since they tell us nothing about what they store.
On the other hand, numApples and numOranges are better names since they do tell us what they’re doing.
Choose Names at the Appropriate Level of Abstraction
We should think about the level of abstraction of our code when we name things with names.
For example, if we have the following Phone class:
class Phone {
dial() {}
}
Then we are being too specific with our naming of the dial method when we actually want to convey that we’re using it to call another person.
Most phones don’t have dials anymore so it wouldn’t really make sense to name them as such. Some phones have keypads and smartphones have screens.
So instead, we should rename it to be more general as follows:
class Phone {
call() {}
}
Unambiguous Names
Names shouldn’t be ambiguous. They shouldn’t have double meaning where people have to guess what the name actually means.
Guessing is bad since people may lead to the wrong conclusion and make mistakes when changing the code.
For example, if we want to create a function to rename a file, we shouldn’t call it rename since it doesn’t tell us what we’re renaming. Instead renameFile is more appropriate since we know for sure that the function is for renaming a file.
Conclusion
We should be clear when we’re naming things. Names should be descriptive and unambiguous.
Also, we should name things at the correct level of abstraction, so names that should be general should be generic, and the opposite is also true.
Transitive navigation is also bad because it creates fragile code. We shouldn’t have a chain of function calls that get different types of objects in order to do something.
Finally, we should encapsulate boundary conditions so we’re clear of what it is and why we have it.
